Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

_{Comment @coderabbitai help to get the list of available commands.}

The docs/testing.md principle is right and worth landing on its own. Three problems with the code change, one of which contradicts the PR's own docstring.

1. The measured mechanism does not reach the observed magnitude

The PR attributes the flake to macOS time.sleep over-sleeping, measured at 0.2 s → 0.24-0.31 s. Run the arithmetic against what CI actually reported:

For three workers sleeping concurrently, elapsed ≈ max(individual sleep). So:

The measured inflation passes the bound with 35% to spare. To reach 0.49-0.57 s while parallel, a single sleep would have to inflate ~2.5-2.9x, and the measurement found 1.2-1.55x. There is a factor of two unaccounted for.

The PR is explicit that the failure was not reproduced ("not reproducible on bare metal even under harsher conditions"). So what was demonstrated is that a mechanism exists which inflates sleeps; what was not demonstrated is that it inflates them enough. "Not a code defect" is the conclusion that justifies deleting the assertion, and that conclusion is not established by this evidence.

The alternative it displaces — partial serialisation on the runner — remains equally unproven, but it is the one that predicts 0.49-0.57 s. It should not be dropped in favour of a mechanism whose measured size is half of what is needed.

2. Dropping the assertion removes the property, and a non-magnitude alternative exists

After this change nothing verifies that the workers run concurrently. If a regression made _SubWorkerPool.dispatch block until completion, this test would pass: three workers, correct results, no errors, serial execution.

The new principle says "Assert the property directly — the functional result or an observable effect — instead." This change asserts only the functional result and drops the property.

Concurrency has a direct, non-magnitude formulation: have each worker write its own start and end timestamps into its mailbox, and assert the three intervals overlap. That is relational, not a bound — it holds whether a sleep takes 0.2 s or 0.6 s, so it is immune to exactly the over-sleeping this PR identifies, while still failing if the pool serialises. It satisfies the principle rather than being an exception to it.

If overlap is rejected for a reason I am not seeing, that reasoning belongs in the PR, because "wall-clock magnitude is a bad proxy" does not by itself imply "assert nothing".

3. The docstring claims coverage the test cannot have

Case 5 — the pool drives N workers at once without cross-talk.
[...] must return its own result

All three workers are registered with _make_sleep_fn(sleep_sec) for the same sleep_sec = 0.2, so every callable returns int(0.2 * 1000) = 200, and the assertion is result == 200 for each. If worker 0's result were routed into worker 1's mailbox the test would still pass.

Cross-talk and result-routing are therefore not covered, and "at once" is precisely what was just removed. Both claims in the new docstring are unsupported by the new test body.

Cheap fix that makes the claim true: give each worker a distinct duration (sleep_sec = 0.1 + 0.05 * i, or just distinct return values) so each result is unique to its worker. That is worth doing regardless of how 1 and 2 are resolved — it is the routing check the docstring already promises.

Suggested split

The docs/testing.md principle is good and I would land it as-is, including the exception clause for tests where wall time genuinely is the subject.

For the test: make the results distinguishable (3), and replace the deleted bound with an interval-overlap assertion (2) rather than nothing. If the overlap assertion then proves flaky on the macOS runner, that is a much stronger result than this PR currently has — it would be direct evidence for serialisation, which is the open question in #1496.

Note that closing #1496 with Fixes #1496 records "flaky proxy, no defect" as the outcome. Given point 1, I do not think the evidence supports that yet.

All three points taken — revised in c19c79e.

1. Magnitude gap — agreed, the claim was unsupported. Measured inflation (0.2 s → 0.24-0.31 s, 1.2-1.55x) gives elapsed ≈ 0.31 s for three parallel sleeps, ~35% under the 0.48 s bound; reaching 0.49-0.57 s parallel needs ~2.5-2.9x, which was never measured. The commit message and PR description now state this plainly: the root cause (worse VM inflation vs. partial serialisation) is open, not "no defect". Nothing is closed.

2. Overlap, not nothing — the deleted bound is replaced by an interval-overlap check, exactly as described. Each worker writes [t_begin, t_end] into its mailbox (offsets 24/32, published before TASK_DONE); the test asserts max(begins) < min(ends). Relational, not a magnitude bound — holds at 0.2 s or 0.6 s, fails if the pool serialises. Verified non-vacuous: forced serial dispatch makes latest_begin > earliest_end, so the assertion trips.

3. Distinct results — each worker now returns a unique tag (100, 200, 300); the result check is per-worker, so a misrouted result fails. The docstring's cross-talk / own-result claim now holds.

Split — docs/testing.md principle kept as-is (including the exception clause). Refs #1496, not Fixes — the issue stays open. If the overlap assertion flakes on the macOS runner, that is direct evidence for serialisation, the open question in #1496.

Re-reviewed c19c79e7. All three points are addressed, and the implementation is correct on the details that could have quietly broken it:

• time.monotonic() is the right clock — system-wide on both Linux and macOS, so the cross-process comparison is valid. perf_counter() would have been wrong here.
• state is still written last in _mb_write_result, after both timestamps, so the parent cannot observe TASK_DONE with stale window values.
• max(begin) < min(end) is the correct test for N intervals sharing a common point, not just pairwise overlap.
• Offsets 24/32 fit inside MAILBOX_SIZE = 256.
• Distinct tag=100*(i+1) makes the routing assertion real — worker 0's result landing in worker 1's mailbox now fails.

The overlap assertion is strictly stronger than what it replaces: it fails on any serialisation, where the old bound only fired when serialisation was severe enough to cross 0.48 s. If the runner really does serialise at ~2%, this should catch it at ≥2%, with a message that says so directly.

One thing left: Fixes #1496 still overstates the outcome

The magnitude claim is withdrawn from the body, which is right. But auto-closing #1496 records the conclusion "flaky proxy, no defect", and this PR does not establish that — it replaces an indirect assertion with a direct one and has not yet seen the direct one survive a macOS run.

I would still close #1496, but for a reason the evidence actually supports, and it is a stronger reason than the one currently claimed:

_SubWorkerPool is defined in this test file and never imports simpler.worker. Whatever the macOS runner does to three forked processes, there is no simpler code path implicated — the old assertion was measuring a host-scheduler property through a POC fixture. That is true regardless of whether the serialisation is real, which is exactly why it is the better reason to close on.

Concretely, I would either:

• reword the body so the close reason is "the assertion tested a host-scheduler property through a test-local fixture; replaced with a direct relational assertion", not "no defect, timer artifact"; or
• drop Fixes #1496, merge, and close [Bug] test_parallel_wall_time: forked workers run near-serially on macOS CI (~2% of runs) #1496 manually once the macOS job has run this green a few times.

Either is fine. What I would avoid is leaving "macOS time.sleep over-sleeping caused it" as the recorded answer, since the factor-of-two gap was never closed and someone will read that later as settled.

And if the overlap assertion ever does fail on macOS — reopen #1496 with that failure attached. It would be the first direct evidence of serialisation, which is more than the original investigation ever produced.

Approving on the code. The only change I am asking for is to the close framing.

ut-a5 fails on this PR and on the re-run with OSError: Environment variable 'ASCEND_HOME_PATH' is not set. in two a5 hardware tests (test_dynamic_alloc_hw.py, test_platform_comm.py). Flagging rather than ignoring (discipline: a red check is not silently "flaky"):

• Unrelated to this PR. The change is a pure-Python fork/shm test + a doc. test_hostsub_fork_shm.py isn't selected in ut-a5 (the run selects 4 a5-hw tests, 971 deselected), and the sibling ut-a2a3 passes.
• Reproduced on re-run — same ASCEND_HOME_PATH error; the a5 toolchain env isn't provisioned on the runner.
• Looks like known infra — there's a ci-a5-test-runner branch with its own failing runs.
• This change's own verdict is green: ut (macos-latest) and ut (ubuntu-latest) both pass.

Failing run: https://github.com/hw-native-sys/simpler/actions/runs/30233519385/job/89879523505